Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

reCAPTCHA: Spacing and template refactor #2588

Open
wants to merge 14 commits into
base: feat/2541-recaptcha-add-link-hide-flag
Choose a base branch
from

Conversation

machikoyasuda
Copy link
Member

@machikoyasuda machikoyasuda commented Dec 12, 2024

closes #2540

This ticket is mostly a code clean-up ticket, aside from the one actual CSS change in this PR (d671ec7 - which this in it of itself is only bumping something 2 pixels), made possible by all the changes already completed in #2585 #2584 #2573 #2538 #2586. This is mostly for optimizing developer experience in the templates.

What this PR does

  • H1s: Are now all 72px from the bottom of the nav. As declared by CSS on the H1 element. This is used on all pages except for the Index page and Error/Logged Out pages.
  • Base: Refactors the base template to combine some of the blocks into 1 wrapper div, instead of mulitple. Removes explanatory-info block to further simplify. There are 2 rows: the main content row, and the all to action row. Within the main content row, it's either headline or inner-content. That's it. There's 64px between these two rows.
  • Naming Conventions: Renames the index templates to follow this new convention - index-base, index--agency-base, index--cst, index--mst... This makes it easier for developers to look at a file and see what template is inheriting from where.

Documentation

  • How to decide whether to use the base template directly, whether to create a new template inheriting from base like error-base, or to create a new template that overrides almost everything.
  • Is the entire page wrapped in a centered col-lg-6? Use base. Use headline, inner-content and call-to-action-button. Do not create any new parent blocks. Examples: Logged Out, Confirm.
  • Is the entire page wrapped in a centered col-lg-6, but there are different templates varying by agency or flow? Create a new template that inherits from base, end the name of this file with base, like success-base. In this new file, use headline for the title and use the inner-content block for your content, and use call-to-action-button for the button. You can create blocks that are children of inner-content, but do not create any new parent blocks. When creating files for each agency or flow, try to make sure that the files are only content, like copy or images and basic HTML elements, and put all styling in the template. Examples: Elig Start, Elig Index.
  • Does the page have a wider or narrower column-width? Create a new template and override main-content. If it feels hacky to use inner-content and call-to-action, create a new template. Examples: Help, Enrollment Success.

Notes

  • If you have col-lg-6 you don't need col-12 col-sm-12. 12 is the default.

Ideals

  • Do not send empty divs into the code. If the page does not have a call-to-action, add this:
{% block call-to-action %}
{% endblock call-to-action %}

@github-actions github-actions bot added back-end Django views, sessions, middleware, models, migrations etc. tests Related to automated testing (unit, UI, integration, etc.) front-end HTML/CSS/JavaScript and Django templates and removed tests Related to automated testing (unit, UI, integration, etc.) labels Dec 12, 2024
Copy link

github-actions bot commented Dec 12, 2024

Coverage report

Click to see where and how coverage changed

FileStatementsMissingCoverageCoverage
(new stmts)
Lines missing
  benefits/core
  views.py
Project Total  

This report was generated by python-coverage-comment-action

@machikoyasuda machikoyasuda self-assigned this Dec 12, 2024
@machikoyasuda machikoyasuda force-pushed the feat/2540-recaptcha-realign branch from 3a72124 to 6fceafb Compare December 12, 2024 19:13
@machikoyasuda machikoyasuda force-pushed the feat/2540-recaptcha-realign branch from 6fceafb to 5b76baa Compare December 16, 2024 18:12
@machikoyasuda machikoyasuda changed the base branch from refactor/recaptcha-copy to feat/2541-recaptcha-add-link-hide-flag December 16, 2024 20:54
@machikoyasuda
Copy link
Member Author

Open question: Would it be useful to have information like #2588 (comment) and #2585 (comment) written up in docs somewhere? It would be documenting what base.html does, and how to override each block (or when to create a new sub-base template). @thekaveman

@machikoyasuda machikoyasuda changed the base branch from feat/2541-recaptcha-add-link-hide-flag to refactor/recaptcha-copy December 17, 2024 01:07
@machikoyasuda machikoyasuda changed the base branch from refactor/recaptcha-copy to feat/2541-recaptcha-add-link-hide-flag December 17, 2024 02:00
@machikoyasuda machikoyasuda requested a review from a team as a code owner December 17, 2024 06:24
@machikoyasuda machikoyasuda changed the title WIP WIP WIP reCAPTCHA: Base and spacing refactor Dec 17, 2024
@machikoyasuda machikoyasuda changed the title reCAPTCHA: Base and spacing refactor reCAPTCHA: Spacing and template refactor Dec 17, 2024
@machikoyasuda machikoyasuda linked an issue Dec 19, 2024 that may be closed by this pull request
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
back-end Django views, sessions, middleware, models, migrations etc. front-end HTML/CSS/JavaScript and Django templates
Projects
None yet
Development

Successfully merging this pull request may close these issues.

reCAPTCHA: Text padding realignment
1 participant